Skip to content

React liveness/provide default device info #6633

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 26 commits into
base: main
Choose a base branch
from

Conversation

riasatali42
Copy link

Description of changes

This PR introduces features to pass in and out the device information (device Id, device label, kind, group Id) for the camera of the liveness check. Also, provides support to add callback methods for camera changes or default device not found.

Key additions:

  • Pass Default device id and label so that the device will be automatically selected.
  • If device Id and device label both are present, device label will be prioritized.
  • Pass out device info (device Id, device label, group Id, kind) once the liveness check is completed.
  • Callback method added for default device not found if the device with the passed in device id/ label not found.
  • Callback method added for camera changes if the camera is changed. So that we can track the updated camera device with device info.

Issue ##6587, Pass deviceId in and out of liveness check

Description of how you validated changes

  1. Created unit tests to test the added features.

Checklist

  • Have read the Pull Request Guidelines
  • PR description included
  • yarn test passes and tests are updated/added
  • PR title and commit messages follow conventional commit syntax
  • If this change should result in a version bump, changeset added (This can be done after creating the PR.) This does not apply to changes made to docs, e2e, examples, or other private packages.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@riasatali42 riasatali42 requested a review from a team as a code owner July 22, 2025 12:59
Copy link

changeset-bot bot commented Jul 22, 2025

🦋 Changeset detected

Latest commit: 3662944

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@aws-amplify/ui-react-liveness Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@Simone319 Simone319 added run-tests Adding this label will trigger tests to run and removed external-contributor labels Jul 22, 2025
@Simone319
Copy link

Hello @riasatali42 , thanks for creating the PR! There's conflicts in the branch. Could you resolve the conflict and resubmit the PR?

@riasatali42
Copy link
Author

Hello @Simone319 , I have updated the branch with the current main branch and resolved all the conflicts. I've updated this PR instead of creating a new branch. Please let me know if I should do otherwise.

@riasatali42
Copy link
Author

@Simone319, I saw after running the tests that there were some ESLint issues. I've fixed those and updated the branch.
cc. @jasonfill

data: { newDeviceId: selectedDeviceId, newStream },
});
} catch (error: any) {
throw new Error('Error updating device:');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's include the original error message. ex:

throw new Error(`Error updating device: ${error.message}`);

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved.

onCameraChange(deviceInfo);
}
} catch (callbackError) {
// console.error('Error in onCameraChange callback:', callbackError);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, we call still log the error, so that the error is not silently swallowed. However, we need to disable the eslint rule, such as:

// eslint-disable-next-line no-console
console.error('Error in onCameraChange callback:', callbackError);

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved.

Comment on lines 610 to 611
// console.error('Error in onCameraNotFound callback:', callbackError);
// Don't rethrow to prevent state machine from getting stuck

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved.


try {
const deviceInfo = getSelectedDeviceInfo(context);
onUserCancel(deviceInfo);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the use case of deviceInfo in onUserCancel callback?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main goal was to make all the callbacks more consistent. I've discarded it as we won't need this.

@@ -145,6 +145,40 @@ export const LivenessCameraModule = (
recording: 'flashFreshnessColors',
});

// Update the device if the selectedDeviceId changes
React.useEffect(() => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to create a new effect here instead of reusing the existing onCameraChange callback?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can discard that as the onCameraChange is already handling everything. So, I discarded.

const deviceInfo = getSelectedDeviceInfo(context);
onUserCancel(deviceInfo);
} catch (callbackError) {
// console.error('Error in onUserCancel callback:', callbackError);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved.

@riasatali42
Copy link
Author

Hello @Simone319 , I've updated the PR as per the feedback.
cc. @jasonfill

@adrianjoshua-strutt
Copy link
Member

adrianjoshua-strutt commented Aug 12, 2025

Hi @riasatali42,

thank you for addressing our concerns. Could you add end-to-end tests to verify the behavior of your changes?

This guide should help you getting started: https://github.com/aws-amplify/amplify-ui/tree/main/packages/e2e

If you got any questions, feel free to reach out.

Thank you!

@riasatali42
Copy link
Author

Hello @adrianjoshua-strutt ,
I followed all the steps to configure and successfully pushed to amplify using "amplify push". But when I'm running the tests, I'm seeing this issue on this API endpoint:
api: POST /dev/livenessnolight/create?challengeType=FaceMovementAndLightChallenge
RESPONSE: 502 {"message": "Internal server error"}

Could you please guide me here to resolve this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-tests Adding this label will trigger tests to run
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants